Skip to content

Update bootstrap #1944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GilbertCherrie
Copy link
Member

Update Bootstrap to 4.0.0. Also added popper.js since it is a dependency of bootstrap 4.0.0

@Fryguy Fryguy self-assigned this Jan 22, 2025
@GilbertCherrie GilbertCherrie force-pushed the update_bootstrap branch 2 times, most recently from f773ad8 to 821eb06 Compare March 31, 2025 15:34
@GilbertCherrie GilbertCherrie requested a review from Fryguy March 31, 2025 15:35
@GilbertCherrie
Copy link
Member Author

GilbertCherrie commented Mar 31, 2025

@Fryguy I fixed the conflicts here if you can review again when you have time

@Fryguy
Copy link
Member

Fryguy commented Mar 31, 2025

Why is popper added directly. That is, won't it just come in naturally as a dependency?

@GilbertCherrie
Copy link
Member Author

GilbertCherrie commented Mar 31, 2025

Why is popper added directly. That is, won't it just come in naturally as a dependency?

When I try to run npm start without the popper package in the package.json it throws this error. I'm assuming because it's a peer dependency we need to actually declare it in the package.json.
Screenshot 2025-03-31 at 11 39 34 AM

@Fryguy
Copy link
Member

Fryguy commented Mar 31, 2025

ok, so from what I understand - the better way to declare missing peer dependencies is to use packageExtensions. This allows you to essentially force bootstrap to pull in the dependency (and we don't have to declare it as "our" dependency.

So in the .yarnrc.yml you would add something like:

packageExtensions:
  "bootstrap@*":
    dependencies:
      "popper.js": "~1.16.1"

@GilbertCherrie GilbertCherrie force-pushed the update_bootstrap branch 2 times, most recently from f1be072 to 6bbf4c1 Compare March 31, 2025 19:18
@miq-bot
Copy link
Member

miq-bot commented Apr 23, 2025

This pull request is not mergeable. Please rebase and repush.

@GilbertCherrie GilbertCherrie force-pushed the update_bootstrap branch 2 times, most recently from 5588019 to b96b2a2 Compare June 19, 2025 14:23
@GilbertCherrie
Copy link
Member Author

@Fryguy I fixed the dependency issue if you can review this again when you get a chance.

@GilbertCherrie GilbertCherrie force-pushed the update_bootstrap branch 2 times, most recently from 440078f to d5a61cd Compare June 19, 2025 14:55
@miq-bot
Copy link
Member

miq-bot commented Jun 19, 2025

Checked commit GilbertCherrie@d82b19d with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.62.0, and yamllint
1 file checked, 1 offense detected

**

  • 💣 💥 🔥 🚒 - Linter/Yaml - missing config files

@GilbertCherrie
Copy link
Member Author

@Fryguy I think this pr is good to merge if you can review

@GilbertCherrie
Copy link
Member Author

@elsamaryv please review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants